MPT-21532 Add generic authentication provider system#332
Merged
Conversation
📝 WalkthroughWalkthroughReplaces the ChangesPluggable Authentication and ExtensionFramework Provider
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
albertsola
force-pushed
the
feature/MPT-21532-authentication-providers
branch
3 times, most recently
from
e4a584a to
d80fed6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
tests/e2e/auth/test_sync_extension_framework.py (1)
3-9: ⚡ Quick winAvoid duplicating an existing E2E extension-auth access assertion.
Line 6-9 repeats the same behavior already covered in
tests/e2e/test_access.py(test_extension_framework_access). Keep one copy, or change this test to validate a distinct auth behavior (for example, refresh/re-auth behavior) so E2E runtime and maintenance do not grow with duplicate coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/auth/test_sync_extension_framework.py` around lines 3 - 9, The test_extension_framework_authenticates_request function duplicates the same product access assertion already covered by test_extension_framework_access in tests/e2e/test_access.py. Either remove this entire test to eliminate duplication, or refactor it to validate a distinct authentication behavior such as token refresh or re-authentication scenarios that are not already covered elsewhere. This will prevent E2E test redundancy and maintenance overhead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mpt_api_client/auth/extension_framework.py`:
- Around line 108-116: The _get_sync_service method caches the token service
based on the first request's origin but does not validate that subsequent
requests use the same origin, creating a cross-origin token reuse vulnerability.
Store the origin when the service is first created (when self._sync_service is
None), and on all subsequent calls verify that the current request's origin
matches the stored origin. If the origins do not match, raise an error to
enforce that the auth instance is pinned to a single origin. This fix applies to
the _get_sync_service method as well as the similar pattern referenced at lines
118-126 (likely in another method handling async service creation).
- Around line 79-83: The code automatically retries all HTTP methods after
receiving a 401 response, which can cause duplicate side effects for
non-idempotent requests (POST, PATCH, PUT, DELETE) if the first attempt already
reached business logic before the auth failure surfaced. Check the HTTP method
of the request before retrying: only retry if the method is idempotent (GET,
HEAD, OPTIONS, TRACE are safe to retry). In file
mpt_api_client/auth/extension_framework.py at lines 79-83, add a method check
before the _refresh_sync call and header update. Apply the same fix at the
second location (lines 93-97) where the same 401 handling logic appears. The
request object has a method attribute that indicates the HTTP method being used.
In `@tests/e2e/conftest.py`:
- Around line 59-62: Add explicit validation for required environment variables
before constructing BearerTokenAuthentication instances. For each token
environment variable (MPT_API_TOKEN_VENDOR, MPT_API_TOKEN_CLIENT,
MPT_API_TOKEN_OPERATIONS, MPT_API_TOKEN_EXTENSION), retrieve the value using
os.getenv() and validate that it is not None. If any required token is missing,
raise an exception immediately with a clear error message that includes the
specific environment variable name. Apply this validation at all affected sites:
the main instance at lines 59-62 for MPT_API_TOKEN_VENDOR, and the sibling
instances at lines 68-71, 77-80, 86-89, and 95-98 for the remaining tokens,
ensuring fail-fast behavior with actionable error messages rather than allowing
None values to propagate and cause cryptic 401 errors later.
In `@tests/unit/http/test_jwt.py`:
- Line 43: The long f-string assignment in line 43 of the token variable exceeds
the repository's Jones complexity threshold and violates WPS221. Break up this
line by extracting the _encode_segment function calls into separate intermediate
variables before constructing the final token string, then reassemble the token
from these variables to maintain functionality while reducing line complexity
and improving readability.
In `@tests/unit/resources/integration/test_installations_token.py`:
- Around line 25-27: Fix the flake8 failures in the test module by addressing
two issues: (1) At line 25 in the test_token_endpoint function, add an explicit
Act step variable by assigning the result of installations_token_service.path to
a variable before asserting it (fixing AAA01 violation for proper
Arrange-Act-Assert pattern), and (2) At lines 40, 52, and 64, flatten any deep
chained member access by breaking them into separate intermediate variables
instead of accessing nested properties directly in one chain (fixing WPS219
complexity violations).
---
Nitpick comments:
In `@tests/e2e/auth/test_sync_extension_framework.py`:
- Around line 3-9: The test_extension_framework_authenticates_request function
duplicates the same product access assertion already covered by
test_extension_framework_access in tests/e2e/test_access.py. Either remove this
entire test to eliminate duplication, or refactor it to validate a distinct
authentication behavior such as token refresh or re-authentication scenarios
that are not already covered elsewhere. This will prevent E2E test redundancy
and maintenance overhead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: b9de982c-7d69-4085-a3f6-1aa13a99787c
📒 Files selected for processing (38)
.env.sampleREADME.mddocs/architecture.mddocs/e2e_tests.mddocs/rql.mddocs/usage.mdmpt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/auth/base.pympt_api_client/auth/extension_framework.pympt_api_client/http/__init__.pympt_api_client/http/async_client.pympt_api_client/http/client.pympt_api_client/http/jwt.pympt_api_client/mpt_client.pympt_api_client/resources/integration/installations_token.pympt_api_client/resources/integration/integration.pypyproject.tomltests/e2e/auth/__init__.pytests/e2e/auth/conftest.pytests/e2e/auth/test_async_extension_framework.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/conftest.pytests/e2e/integration/installations_token/__init__.pytests/e2e/integration/installations_token/conftest.pytests/e2e/integration/installations_token/test_async_installations_token.pytests/e2e/integration/installations_token/test_sync_installations_token.pytests/e2e/test_access.pytests/unit/auth/__init__.pytests/unit/auth/test_base.pytests/unit/auth/test_extension_framework.pytests/unit/conftest.pytests/unit/http/test_async_client.pytests/unit/http/test_client.pytests/unit/http/test_jwt.pytests/unit/resources/integration/test_installations_token.pytests/unit/resources/integration/test_integration.pytests/unit/test_mpt_client.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
softwareone-platform/mpt-extension-skills(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep
README.mdconcise and navigational in the repository root
Files:
README.md
**/*
⚙️ CodeRabbit configuration file
**/*: For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved
Files:
README.mdtests/unit/auth/test_base.pydocs/rql.mddocs/e2e_tests.mdtests/e2e/auth/conftest.pytests/unit/http/test_client.pytests/unit/http/test_async_client.pympt_api_client/auth/__init__.pytests/e2e/integration/installations_token/test_sync_installations_token.pympt_api_client/http/__init__.pytests/unit/resources/integration/test_integration.pytests/e2e/auth/test_async_extension_framework.pytests/unit/http/test_jwt.pytests/e2e/integration/installations_token/test_async_installations_token.pympt_api_client/resources/integration/installations_token.pytests/e2e/auth/test_sync_extension_framework.pydocs/architecture.mdmpt_api_client/http/jwt.pytests/e2e/test_access.pympt_api_client/http/client.pympt_api_client/resources/integration/integration.pytests/e2e/integration/installations_token/conftest.pydocs/usage.mdtests/unit/resources/integration/test_installations_token.pytests/unit/conftest.pympt_api_client/http/async_client.pympt_api_client/mpt_client.pympt_api_client/__init__.pympt_api_client/auth/base.pympt_api_client/auth/extension_framework.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pypyproject.tomltests/unit/test_mpt_client.py
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdWorking protocol for any task in this repository:
- Identify the task type and select only the local repository files that are relevant to that task.
- Read only those relevant local files before making changes.
- If any selected local file references shared standards or shared operational guidance that are relevant to the same task, read those shared documents too before proceeding.
- Treat repository-local documents as repository-specific additions, restrictions, or overrides to shared guidance.
- If a repository-local rule conflicts with a shared rule, the local repository rule takes precedence.
Python API client for the SoftwareONE Marketplace Platform (MPT) API. Provides synchronous
(MPTClient) and asynchronous (AsyncMPTClient) clients built on httpx, with typed
resource services, mixin-based HTTP operations, and an RQL query builder.Documentation Reading Order
When applicable, read the repository documentation in this order:
README.md— repository overview, quick start, and documentation mapdocs/usage.md— installation, configuration, Python usage examples, and supported Docker-based commandsdocs/architecture.md— layered architecture, directory structure, and key abstractionsdocs/local-development.md— Docker-only setup and execution modeldocs/testing.md— repository-specific testing strategy and command mappingdocs/contributing.md— repository-specific workflow and links to shared standardsdocs/documentation.md— repository-specific documentation rulesdocs/unit_tests.md— unit test structure and guidancedocs/e2e_tests.md— end-to-end test setup and executionThen inspect the code paths relevant to the task:
mpt_api_client/mpt_client.py— public sync and async client entry pointsmpt_api_client/http/— HTTP clients, services, query state, and reusable mixinsmpt_api_client/resources/— domain resource groups such as catalog, commerce, billing, and integrati...
Files:
README.mdtests/unit/auth/test_base.pydocs/rql.mddocs/e2e_tests.mdtests/e2e/auth/conftest.pytests/unit/http/test_client.pytests/unit/http/test_async_client.pympt_api_client/auth/__init__.pytests/e2e/integration/installations_token/test_sync_installations_token.pympt_api_client/http/__init__.pytests/unit/resources/integration/test_integration.pytests/e2e/auth/test_async_extension_framework.pytests/unit/http/test_jwt.pytests/e2e/integration/installations_token/test_async_installations_token.pympt_api_client/resources/integration/installations_token.pytests/e2e/auth/test_sync_extension_framework.pydocs/architecture.mdmpt_api_client/http/jwt.pytests/e2e/test_access.pympt_api_client/http/client.pympt_api_client/resources/integration/integration.pytests/e2e/integration/installations_token/conftest.pydocs/usage.mdtests/unit/resources/integration/test_installations_token.pytests/unit/conftest.pympt_api_client/http/async_client.pympt_api_client/mpt_client.pympt_api_client/__init__.pympt_api_client/auth/base.pympt_api_client/auth/extension_framework.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pypyproject.tomltests/unit/test_mpt_client.py
docs/**/*.md
📄 CodeRabbit inference engine (docs/contributing.md)
When repository behavior changes, update the narrowest relevant document under
docs/
Files:
docs/rql.mddocs/e2e_tests.mddocs/architecture.mddocs/usage.md
docs/*.md
📄 CodeRabbit inference engine (docs/documentation.md)
docs/*.md: Topic-specific documentation must live in the matching file underdocs/directory
Shared engineering rules must be linked frommpt-extension-skillsinstead of copied into this repository
Files:
docs/rql.mddocs/e2e_tests.mddocs/architecture.mddocs/usage.md
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
docs/**: Put topic-specific documentation underdocs/directory instead of expandingREADME.md
Link shared engineering rules frommpt-extension-skillsinstead of duplicating them locally in documentation
Files:
docs/rql.mddocs/e2e_tests.mddocs/architecture.mddocs/usage.md
⚙️ CodeRabbit configuration file
docs/**: Review documentation changes againstdocs/documentation.mdand the linked repository'sstandards/documentation.md.
Use those documents as the source of truth for structure, topic boundaries, navigation updates, and when to link shared rules instead of copying them.
Files:
docs/rql.mddocs/e2e_tests.mddocs/architecture.mddocs/usage.md
⚙️ CodeRabbit configuration file
docs/**: # ArchitectureThis document describes the internal architecture of
mpt-api-python-client.Overview
mpt-api-python-clientis a Python API client that provides a typed, fluent interface for the
SoftwareONE Marketplace Platform (MPT) REST API. It supports both synchronous and asynchronous
usage and is built on top of httpx.API Reference: The full upstream API contract is described by the
MPT OpenAPI Spec.
The client mirrors this spec's resource structure.The client exposes every MPT API domain (catalog, commerce, billing, etc.) as a resource group,
where each resource is a service object composed from reusable HTTP operation mixins.Directory Structure
mpt_api_client/ ├── __init__.py # Public API: MPTClient, AsyncMPTClient, RQLQuery ├── mpt_client.py # Client entry points ├── constants.py # Shared constants (content types) ├── exceptions.py # Error hierarchy (MPTError, MPTHttpError, MPTAPIError) │ ├── http/ # HTTP transport layer │ ├── client.py # Sync HTTPClient (httpx.Client) │ ├── async_client.py # Async AsyncHTTPClient (httpx.AsyncClient) │ ├── base_service.py # ServiceBase — shared service logic │ ├── service.py # Service — sync service (extends ServiceBase) │ ├── async_service.py # AsyncService — async service (extends ServiceBase) │ ├── query_state.py # Query parameter accumulation │ ├── client_utils.py # URL validation helpers │ ├── types.py # Type aliases (Response, HeaderTypes, etc.) │ └── mixins/ # Composable HTTP operation mixins │ ├── collection_mixin.py │ ├── create_mixin.py │ ├── create_file_mixin.py │ ├── update_mixin.py │ ├── update_file_mixin.py │ ├── delete_mixin.py │ ├── get_mixin.py │ ├── enable_mixin.py │ ├── disable_mixin...
Files:
docs/rql.mddocs/e2e_tests.mddocs/architecture.mddocs/usage.md
docs/usage.md
📄 CodeRabbit inference engine (docs/documentation.md)
docs/usage.mdis the source of truth for installation, configuration, examples, and supported command entry points
Files:
docs/usage.md
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:23.203Z
Learning: The client architecture should be organized in four layers: Client Layer (entry point), Resource Groups (domains), Service + Mixins (HTTP operations), and HTTPClient/AsyncHTTPClient (transport)
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:23.203Z
Learning: Services should mirror the upstream MPT OpenAPI Spec's resource structure
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:23.203Z
Learning: The client should support both synchronous and asynchronous usage patterns
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:38.416Z
Learning: End-to-end tests should exercise the running MPT API and cover the full request/response lifecycle
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:38.416Z
Learning: E2E tests must rely on live credentials and configurable endpoints
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:38.416Z
Learning: Required environment variables for E2E tests are: `MPT_API_BASE_URL`, `MPT_API_TOKEN_VENDOR`, `MPT_API_TOKEN_CLIENT`, `MPT_API_TOKEN_OPERATIONS`, and `MPT_API_TOKEN_EXTENSION`
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:38.416Z
Learning: Optional ReportPortal integration can be configured using `RP_API_KEY`, `RP_ENDPOINT`, and `RP_LAUNCH` environment variables
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:38.416Z
Learning: E2E tests should be run outside the default `make test` invocation and require explicit specification via `make test args="tests/e2e"`
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:55.045Z
Learning: Refer to rql.md for the authoritative RQL syntax and query builder usage documentation
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:55.045Z
Learning: Refer to architecture.md for repository structure and abstraction details
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:55.045Z
Learning: Refer to testing.md for validation and test command behavior
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T14:53:55.045Z
Learning: Refer to local-development.md for repository-local Docker workflow for contributors
📚 Learning: 2026-04-16T11:49:53.579Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 303
File: tests/e2e/helpdesk/chats/participants/conftest.py:25-31
Timestamp: 2026-04-16T11:49:53.579Z
Learning: In tests/e2e/helpdesk/chats/participants/conftest.py (E2E fixtures `created_chat_participant` and `async_created_chat_participant`), it’s acceptable to omit participant teardown/cleanup. The parent `created_chat` fixture performs cascade-deletion of the chat and all related records (including participants) during teardown, so missing participant-specific cleanup should not be flagged in code review.
Applied to files:
.env.sample
📚 Learning: 2025-12-12T15:02:20.732Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 160
File: tests/e2e/commerce/agreement/attachment/test_async_agreement_attachment.py:55-58
Timestamp: 2025-12-12T15:02:20.732Z
Learning: In pytest with pytest-asyncio, if a test function uses async fixtures but contains no await, declare the test function as def (synchronous) instead of async def. Pytest-asyncio will resolve the async fixtures automatically; this avoids linter complaints about unnecessary async functions. This pattern applies to any test file under the tests/ directory that uses such fixtures.
Applied to files:
tests/unit/auth/test_base.pytests/e2e/auth/conftest.pytests/unit/http/test_client.pytests/unit/http/test_async_client.pytests/e2e/integration/installations_token/test_sync_installations_token.pytests/unit/resources/integration/test_integration.pytests/e2e/auth/test_async_extension_framework.pytests/unit/http/test_jwt.pytests/e2e/integration/installations_token/test_async_installations_token.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/test_access.pytests/e2e/integration/installations_token/conftest.pytests/unit/resources/integration/test_installations_token.pytests/unit/conftest.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/test_mpt_client.py
📚 Learning: 2026-04-02T09:35:03.825Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 269
File: tests/e2e/helpdesk/chats/links/test_sync_links.py:18-18
Timestamp: 2026-04-02T09:35:03.825Z
Learning: In this repository’s test suite, flake8-aaa/flake8-aaa codes use short two-digit suffixes (e.g., `# noqa: AAA01`), not three-digit variants like `AAA001`. If you see `# noqa: AAA01` in a test file (e.g., when the Act step is performed via a pytest fixture rather than inline code), treat it as valid and intentional—do not flag it as an invalid/no-longer-needed noqa, and do not suggest removing it even if Ruff reports `RUF102`, since `AAA` is configured under `tool.ruff.lint.external` and these noqa directives are expected to be preserved.
Applied to files:
tests/unit/auth/test_base.pytests/e2e/auth/conftest.pytests/unit/http/test_client.pytests/unit/http/test_async_client.pytests/e2e/integration/installations_token/test_sync_installations_token.pytests/unit/resources/integration/test_integration.pytests/e2e/auth/test_async_extension_framework.pytests/unit/http/test_jwt.pytests/e2e/integration/installations_token/test_async_installations_token.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/test_access.pytests/e2e/integration/installations_token/conftest.pytests/unit/resources/integration/test_installations_token.pytests/unit/conftest.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/test_mpt_client.py
📚 Learning: 2026-02-02T13:05:41.144Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 201
File: tests/unit/resources/accounts/mixins/test_activatable_mixin.py:132-139
Timestamp: 2026-02-02T13:05:41.144Z
Learning: In the mpt-api-python-client repository, tests are configured to use pytest asyncio mode auto, which auto-detects async test functions and runs them without requiring pytest.mark.asyncio. Reviewers should rely on this behavior for all Python test files under tests/, and avoid adding unnecessary asyncio markers in async tests. Ensure test files in tests/ adhere to this convention unless a specific test requires an explicit marker.
Applied to files:
tests/unit/auth/test_base.pytests/e2e/auth/conftest.pytests/unit/http/test_client.pytests/unit/http/test_async_client.pytests/e2e/integration/installations_token/test_sync_installations_token.pytests/unit/resources/integration/test_integration.pytests/e2e/auth/test_async_extension_framework.pytests/unit/http/test_jwt.pytests/e2e/integration/installations_token/test_async_installations_token.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/test_access.pytests/e2e/integration/installations_token/conftest.pytests/unit/resources/integration/test_installations_token.pytests/unit/conftest.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/test_mpt_client.py
📚 Learning: 2026-04-16T13:00:41.320Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 303
File: tests/e2e/helpdesk/chats/participants/conftest.py:25-31
Timestamp: 2026-04-16T13:00:41.320Z
Learning: In mpt-api-python-client, do not treat list-wrapped arguments to CreateMixin.create() / AsyncCreateMixin.create() as an error. ResourceData is intentionally typed as Resource | list[Resource] (see mpt_api_client/models/model.py), so create() should accept either a single resource dict or a list of resource dicts (e.g., create([chat_participant_data])) to perform a batch create and return a ModelCollection. Therefore, reviewers should only flag list-wrapped create() arguments when there is evidence they violate the expected API contract beyond this documented batch-create behavior.
Applied to files:
tests/unit/auth/test_base.pytests/e2e/auth/conftest.pytests/unit/http/test_client.pytests/unit/http/test_async_client.pympt_api_client/auth/__init__.pytests/e2e/integration/installations_token/test_sync_installations_token.pympt_api_client/http/__init__.pytests/unit/resources/integration/test_integration.pytests/e2e/auth/test_async_extension_framework.pytests/unit/http/test_jwt.pytests/e2e/integration/installations_token/test_async_installations_token.pympt_api_client/resources/integration/installations_token.pytests/e2e/auth/test_sync_extension_framework.pympt_api_client/http/jwt.pytests/e2e/test_access.pympt_api_client/http/client.pympt_api_client/resources/integration/integration.pytests/e2e/integration/installations_token/conftest.pytests/unit/resources/integration/test_installations_token.pytests/unit/conftest.pympt_api_client/http/async_client.pympt_api_client/mpt_client.pympt_api_client/__init__.pympt_api_client/auth/base.pympt_api_client/auth/extension_framework.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/test_mpt_client.py
📚 Learning: 2026-01-08T08:34:05.465Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 183
File: tests/e2e/catalog/price_lists/conftest.py:29-30
Timestamp: 2026-01-08T08:34:05.465Z
Learning: In end-to-end tests (e.g., tests/e2e/...), reuse existing API resources for read-only operations and safe mutations to speed up test execution. Reserve isolated fixtures (e.g., created_price_list) for destructive tests that require per-test creation and cleanup. Ensure tests document when a fixture creates/destroys data and clearly indicate which operations are destructive, so tests stay fast and properly isolated.
Applied to files:
tests/e2e/auth/conftest.pytests/e2e/integration/installations_token/test_sync_installations_token.pytests/e2e/auth/test_async_extension_framework.pytests/e2e/integration/installations_token/test_async_installations_token.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/test_access.pytests/e2e/integration/installations_token/conftest.pytests/e2e/conftest.py
📚 Learning: 2026-01-08T23:38:19.565Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 186
File: tests/e2e/billing/ledger/charge/test_sync_ledger_charge.py:33-39
Timestamp: 2026-01-08T23:38:19.565Z
Learning: In Python e2e tests under tests/e2e, hardcoded external IDs (e.g., INV12345) are intentional because they come from seeded test data. Ensure the test data seeds consistently include these IDs; if seeds change, update tests accordingly. Prefer using a named constant for such IDs (e.g., INV_EXTERNAL_ID) and document the dependency on seed data to avoid brittle tests.
Applied to files:
tests/e2e/auth/conftest.pytests/e2e/integration/installations_token/test_sync_installations_token.pytests/e2e/auth/test_async_extension_framework.pytests/e2e/integration/installations_token/test_async_installations_token.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/test_access.pytests/e2e/integration/installations_token/conftest.pytests/e2e/conftest.py
📚 Learning: 2026-04-02T16:26:46.138Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 275
File: tests/e2e/exchange/currencies/conftest.py:43-43
Timestamp: 2026-04-02T16:26:46.138Z
Learning: In this repo’s Python E2E conftest files, line-specific flake8/wemake suppressions like `# noqa: WPS421` placed inline on individual statements (e.g., on specific `print()` calls in teardown blocks) are intentional and acceptable. When reviewing, avoid suggesting converting these to a file-wide ignore (e.g., adding per-file ignores under `[tool.flake8]` in `pyproject.toml`), since that would over-suppress WPS421 for the entire file. Also treat Ruff `RUF102` about an unknown rule code as expected here when suppressing `WPS421` from wemake-python-styleguide.
Applied to files:
tests/e2e/auth/conftest.pytests/e2e/integration/installations_token/conftest.pytests/e2e/conftest.py
📚 Learning: 2026-04-06T09:03:34.466Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 277
File: tests/e2e/extensibility/extensions/conftest.py:37-40
Timestamp: 2026-04-06T09:03:34.466Z
Learning: In this repo’s E2E test conftest teardown fixtures, it is intentional to catch `MPTAPIError` broadly (not just 404) and to print a diagnostic message instead of re-raising. This avoids failing the test suite during teardown when a resource cannot be deleted in some scenarios. Do not recommend narrowing the `except` clause in these teardown blocks (e.g., checking `error.status == 404`), unless the fixture is clearly not intended as an E2E teardown diagnostic.
Applied to files:
tests/e2e/auth/conftest.pytests/e2e/integration/installations_token/conftest.pytests/e2e/conftest.py
📚 Learning: 2026-02-17T10:04:00.873Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 210
File: mpt_api_client/rql/query_builder.py:18-18
Timestamp: 2026-02-17T10:04:00.873Z
Learning: In this repository, Ruff and flake8 with wemake-python-styleguide are used together. Do not remove WPS* noqa directives (e.g., WPS231) even if Ruff flags them as unknown in RUF102. Keep the directives to satisfy flake8 rules; ensure tooling configuration accounts for both linters to avoid false positives.
Applied to files:
mpt_api_client/auth/__init__.pympt_api_client/http/__init__.pympt_api_client/resources/integration/installations_token.pympt_api_client/http/jwt.pympt_api_client/http/client.pympt_api_client/resources/integration/integration.pympt_api_client/http/async_client.pympt_api_client/mpt_client.pympt_api_client/__init__.pympt_api_client/auth/base.pympt_api_client/auth/extension_framework.py
🪛 ast-grep (0.43.0)
tests/unit/http/test_jwt.py
[info] 13-13: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: Security best practice.
(use-jsonify)
tests/unit/auth/test_extension_framework.py
[info] 20-20: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: Security best practice.
(use-jsonify)
🪛 GitHub Actions: PR build and merge / 0_build.txt
tests/unit/http/test_jwt.py
[error] 43-43: flake8 (wemake-python-styleguide) violation WPS221: Found line with high Jones Complexity: 15 > 14.
tests/unit/resources/integration/test_installations_token.py
[error] 25-25: flake8 (wemake-python-styleguide) violation AAA01: no Act block found in test.
[error] 40-40: flake8 (wemake-python-styleguide) violation WPS219: Found too deep access level: 5 > 4.
[error] 52-52: flake8 (wemake-python-styleguide) violation WPS219: Found too deep access level: 6 > 4.
[error] 64-64: flake8 (wemake-python-styleguide) violation WPS219: Found too deep access level: 6 > 4.
🪛 GitHub Actions: PR build and merge / build
tests/unit/http/test_jwt.py
[error] 43-43: flake8 (wemake-python-styleguide) violation WPS221: Found line with high Jones Complexity: 15 > 14.
tests/unit/resources/integration/test_installations_token.py
[error] 25-25: flake8 (wemake-python-styleguide) violation AAA01: no Act block found in test.
[error] 40-40: flake8 (wemake-python-styleguide) violation WPS219: Found too deep access level: 5 > 4.
[error] 52-52: flake8 (wemake-python-styleguide) violation WPS219: Found too deep access level: 6 > 4.
[error] 64-64: flake8 (wemake-python-styleguide) violation WPS219: Found too deep access level: 6 > 4.
🪛 Ruff (0.15.17)
mpt_api_client/auth/__init__.py
[warning] 4-4: Invalid rule code in # noqa: WPS410
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
mpt_api_client/http/__init__.py
[warning] 11-11: Invalid rule code in # noqa: WPS410
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
tests/unit/http/test_jwt.py
[error] 36-36: Possible hardcoded password assigned to: "token"
(S105)
tests/unit/resources/integration/test_installations_token.py
[error] 38-38: Possible hardcoded password assigned to: "token"
(S105)
[error] 51-51: Possible hardcoded password assigned to: "token"
(S105)
[error] 63-63: Possible hardcoded password assigned to: "token"
(S105)
mpt_api_client/__init__.py
[warning] 9-9: Invalid rule code in # noqa: WPS410
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
tests/e2e/conftest.py
[warning] 48-48: Invalid rule code in # noqa: WPS421
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
tests/unit/auth/test_extension_framework.py
[error] 14-14: Possible hardcoded password assigned to: "SECRET"
(S105)
🔇 Additional comments (33)
.env.sample (1)
1-7: ⚡ Quick winConsider removing
MPT_API_TOKENfrom the sample if it is truly unsupported.The PR objectives state that the
MPT_API_TOKENenvironment variable is "no longer supported," yet line 2 of.env.samplestill declares it. The README's quick-start comment was updated to mention onlyMPT_API_BASE_URL, and all downstream documentation (usage.md, e2e_tests.md, rql.md) never mentionMPT_API_TOKEN—only the new explicitAuthenticationproviders.If the library no longer reads
MPT_API_TOKEN, the sample template should be updated to remove or deprecate this line to avoid confusing users into configuring a variable that is ignored. Alternatively, if the variable is intentionally retained for backwards compatibility or other purposes, that intent should be documented.README.md (1)
25-31: LGTM!docs/architecture.md (2)
91-107: LGTM!
154-166: LGTM!docs/e2e_tests.md (1)
31-40: LGTM!docs/rql.md (1)
9-50: LGTM!docs/usage.md (5)
15-39: LGTM!
40-86: LGTM!
90-118: LGTM!
120-142: LGTM!
164-195: LGTM!mpt_api_client/http/jwt.py (1)
1-57: LGTM!mpt_api_client/http/__init__.py (1)
4-8: LGTM!Also applies to: 15-18
tests/unit/http/test_jwt.py (1)
1-42: LGTM!Also applies to: 45-47
mpt_api_client/auth/base.py (2)
1-7: LGTM!Also applies to: 13-28
8-8: No action needed —typing.overrideis compatible with the declared Python version.The repository explicitly requires
Python >=3.12(as declared inpyproject.toml,Dockerfile, and.python-version). Thetyping.overrideimport at line 8 is valid and supported in Python 3.12+.mpt_api_client/auth/__init__.py (1)
1-8: LGTM!mpt_api_client/__init__.py (1)
1-5: LGTM!Also applies to: 9-16
tests/unit/auth/test_base.py (1)
1-14: LGTM!mpt_api_client/http/client.py (1)
7-7: LGTM!Also applies to: 35-35, 50-51
mpt_api_client/http/async_client.py (1)
6-6: LGTM!Also applies to: 25-25, 40-41
tests/unit/http/test_client.py (1)
8-8: LGTM!Also applies to: 17-17, 19-19, 24-25, 31-31, 35-35, 40-41
tests/unit/http/test_async_client.py (1)
8-8: LGTM!Also applies to: 27-27, 29-29, 34-35, 41-41, 45-45, 50-51
mpt_api_client/mpt_client.py (1)
3-3: LGTM!Also applies to: 36-38, 43-43, 50-50, 58-60, 123-125, 130-130, 137-137, 145-145
tests/unit/test_mpt_client.py (1)
3-3: LGTM!Also applies to: 34-36, 56-58, 66-66, 69-71, 94-96, 104-104, 107-109
tests/unit/conftest.py (1)
3-3: LGTM!Also applies to: 17-17, 22-22
tests/e2e/conftest.py (1)
10-11: LGTM!Also applies to: 19-49
tests/e2e/auth/conftest.py (1)
1-25: LGTM!tests/e2e/auth/test_async_extension_framework.py (1)
1-10: LGTM!tests/e2e/integration/installations_token/conftest.py (1)
1-24: LGTM!tests/e2e/integration/installations_token/test_async_installations_token.py (1)
1-18: LGTM!tests/e2e/integration/installations_token/test_sync_installations_token.py (1)
1-16: LGTM!tests/e2e/test_access.py (1)
3-7: LGTM!Also applies to: 13-16, 29-42
albertsola
force-pushed
the
feature/MPT-21532-authentication-providers
branch
2 times, most recently
from
c478a18 to
d15d069
Compare
robcsegal
approved these changes
Jun 16, 2026
albertsola
force-pushed
the
feature/MPT-21532-authentication-providers
branch
from
d15d069 to
b9b9e3c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/auth/test_extension_framework.py (1)
270-303: 💤 Low valueConsider adding async variants of proactive refresh tests.
The sync tests
test_extension_framework_refreshes_proactively_before_expiryandtest_extension_framework_reuses_unexpired_tokenvalidate JWT-expiry-based caching behavior. For symmetry with other test pairs, consider adding async versions to ensure the proactive refresh logic works correctly inasync_auth_flowas well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/auth/test_extension_framework.py` around lines 270 - 303, Add async variants of the two test functions test_extension_framework_refreshes_proactively_before_expiry and test_extension_framework_reuses_unexpired_token to ensure the proactive refresh logic works correctly in the async authentication flow. Create async versions (e.g., test_extension_framework_refreshes_proactively_before_expiry_async and test_extension_framework_reuses_unexpired_token_async) that use the same test setup and assertions but with async/await syntax for any asynchronous client method calls, mirroring the existing sync test logic to validate that token caching and proactive refresh behavior functions properly in async contexts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unit/auth/test_extension_framework.py`:
- Around line 270-303: Add async variants of the two test functions
test_extension_framework_refreshes_proactively_before_expiry and
test_extension_framework_reuses_unexpired_token to ensure the proactive refresh
logic works correctly in the async authentication flow. Create async versions
(e.g., test_extension_framework_refreshes_proactively_before_expiry_async and
test_extension_framework_reuses_unexpired_token_async) that use the same test
setup and assertions but with async/await syntax for any asynchronous client
method calls, mirroring the existing sync test logic to validate that token
caching and proactive refresh behavior functions properly in async contexts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 092c350f-6b8a-405c-a82e-8677b8175c9d
📒 Files selected for processing (38)
.env.sampleREADME.mddocs/architecture.mddocs/e2e_tests.mddocs/rql.mddocs/usage.mdmpt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/auth/base.pympt_api_client/auth/extension_framework.pympt_api_client/http/__init__.pympt_api_client/http/async_client.pympt_api_client/http/client.pympt_api_client/http/jwt.pympt_api_client/mpt_client.pympt_api_client/resources/integration/installations_token.pympt_api_client/resources/integration/integration.pypyproject.tomltests/e2e/auth/__init__.pytests/e2e/auth/conftest.pytests/e2e/auth/test_async_extension_framework.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/conftest.pytests/e2e/integration/installations_token/__init__.pytests/e2e/integration/installations_token/conftest.pytests/e2e/integration/installations_token/test_async_installations_token.pytests/e2e/integration/installations_token/test_sync_installations_token.pytests/e2e/test_access.pytests/unit/auth/__init__.pytests/unit/auth/test_base.pytests/unit/auth/test_extension_framework.pytests/unit/conftest.pytests/unit/http/test_async_client.pytests/unit/http/test_client.pytests/unit/http/test_jwt.pytests/unit/resources/integration/test_installations_token.pytests/unit/resources/integration/test_integration.pytests/unit/test_mpt_client.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
softwareone-platform/mpt-extension-skills(manual)
✅ Files skipped from review due to trivial changes (8)
- docs/e2e_tests.md
- .env.sample
- tests/unit/auth/test_base.py
- docs/architecture.md
- README.md
- docs/rql.md
- docs/usage.md
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (19)
- tests/e2e/integration/installations_token/test_sync_installations_token.py
- mpt_api_client/resources/integration/integration.py
- tests/unit/http/test_async_client.py
- mpt_api_client/auth/base.py
- tests/e2e/auth/conftest.py
- tests/e2e/integration/installations_token/test_async_installations_token.py
- tests/e2e/integration/installations_token/conftest.py
- tests/unit/conftest.py
- tests/unit/http/test_client.py
- tests/unit/resources/integration/test_integration.py
- tests/e2e/test_access.py
- mpt_api_client/http/client.py
- mpt_api_client/resources/integration/installations_token.py
- tests/unit/test_mpt_client.py
- mpt_api_client/http/async_client.py
- tests/e2e/auth/test_async_extension_framework.py
- mpt_api_client/http/jwt.py
- mpt_api_client/mpt_client.py
- tests/e2e/auth/test_sync_extension_framework.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved
Files:
mpt_api_client/auth/extension_framework.pytests/unit/auth/test_extension_framework.pympt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/http/__init__.pytests/e2e/conftest.pytests/unit/http/test_jwt.pytests/unit/resources/integration/test_installations_token.py
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdWorking protocol for any task in this repository:
- Identify the task type and select only the local repository files that are relevant to that task.
- Read only those relevant local files before making changes.
- If any selected local file references shared standards or shared operational guidance that are relevant to the same task, read those shared documents too before proceeding.
- Treat repository-local documents as repository-specific additions, restrictions, or overrides to shared guidance.
- If a repository-local rule conflicts with a shared rule, the local repository rule takes precedence.
Python API client for the SoftwareONE Marketplace Platform (MPT) API. Provides synchronous
(MPTClient) and asynchronous (AsyncMPTClient) clients built on httpx, with typed
resource services, mixin-based HTTP operations, and an RQL query builder.Documentation Reading Order
When applicable, read the repository documentation in this order:
README.md— repository overview, quick start, and documentation mapdocs/usage.md— installation, configuration, Python usage examples, and supported Docker-based commandsdocs/architecture.md— layered architecture, directory structure, and key abstractionsdocs/local-development.md— Docker-only setup and execution modeldocs/testing.md— repository-specific testing strategy and command mappingdocs/contributing.md— repository-specific workflow and links to shared standardsdocs/documentation.md— repository-specific documentation rulesdocs/unit_tests.md— unit test structure and guidancedocs/e2e_tests.md— end-to-end test setup and executionThen inspect the code paths relevant to the task:
mpt_api_client/mpt_client.py— public sync and async client entry pointsmpt_api_client/http/— HTTP clients, services, query state, and reusable mixinsmpt_api_client/resources/— domain resource groups such as catalog, commerce, billing, and integrati...
Files:
mpt_api_client/auth/extension_framework.pytests/unit/auth/test_extension_framework.pympt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/http/__init__.pytests/e2e/conftest.pytests/unit/http/test_jwt.pytests/unit/resources/integration/test_installations_token.py
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:23.652Z
Learning: The client mirrors the MPT OpenAPI Spec's resource structure, exposing every API domain (catalog, commerce, billing, etc.) as a resource group
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:29.116Z
Learning: End-to-end tests must exercise the running MPT API and cover the full request/response lifecycle
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:29.116Z
Learning: Optional ReportPortal integration for E2E tests uses environment variables: RP_API_KEY, RP_ENDPOINT, RP_LAUNCH
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:29.116Z
Learning: E2E test results must be published to Report Portal
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:34.593Z
Learning: Keep filters immutable when composing multiple RQLQuery instances; each `filter()` call returns a new QueryableMixin instance rather than mutating the existing one
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:44.412Z
Learning: Install the mpt-api-client package using `pip install mpt-api-client` or `uv add mpt-api-client`
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:44.412Z
Learning: Refer to rql.md for the authoritative RQL syntax and builder usage documentation
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:44.412Z
Learning: Refer to testing.md for validation and test command behavior
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:44.412Z
Learning: Refer to architecture.md for repository structure and abstractions
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:28:44.412Z
Learning: Refer to local-development.md for repository-local Docker workflow instructions for contributors
📚 Learning: 2026-02-17T10:04:00.873Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 210
File: mpt_api_client/rql/query_builder.py:18-18
Timestamp: 2026-02-17T10:04:00.873Z
Learning: In this repository, Ruff and flake8 with wemake-python-styleguide are used together. Do not remove WPS* noqa directives (e.g., WPS231) even if Ruff flags them as unknown in RUF102. Keep the directives to satisfy flake8 rules; ensure tooling configuration accounts for both linters to avoid false positives.
Applied to files:
mpt_api_client/auth/extension_framework.pympt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/http/__init__.py
📚 Learning: 2026-04-16T13:00:41.320Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 303
File: tests/e2e/helpdesk/chats/participants/conftest.py:25-31
Timestamp: 2026-04-16T13:00:41.320Z
Learning: In mpt-api-python-client, do not treat list-wrapped arguments to CreateMixin.create() / AsyncCreateMixin.create() as an error. ResourceData is intentionally typed as Resource | list[Resource] (see mpt_api_client/models/model.py), so create() should accept either a single resource dict or a list of resource dicts (e.g., create([chat_participant_data])) to perform a batch create and return a ModelCollection. Therefore, reviewers should only flag list-wrapped create() arguments when there is evidence they violate the expected API contract beyond this documented batch-create behavior.
Applied to files:
mpt_api_client/auth/extension_framework.pytests/unit/auth/test_extension_framework.pympt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/http/__init__.pytests/e2e/conftest.pytests/unit/http/test_jwt.pytests/unit/resources/integration/test_installations_token.py
📚 Learning: 2025-12-12T15:02:20.732Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 160
File: tests/e2e/commerce/agreement/attachment/test_async_agreement_attachment.py:55-58
Timestamp: 2025-12-12T15:02:20.732Z
Learning: In pytest with pytest-asyncio, if a test function uses async fixtures but contains no await, declare the test function as def (synchronous) instead of async def. Pytest-asyncio will resolve the async fixtures automatically; this avoids linter complaints about unnecessary async functions. This pattern applies to any test file under the tests/ directory that uses such fixtures.
Applied to files:
tests/unit/auth/test_extension_framework.pytests/e2e/conftest.pytests/unit/http/test_jwt.pytests/unit/resources/integration/test_installations_token.py
📚 Learning: 2026-04-02T09:35:03.825Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 269
File: tests/e2e/helpdesk/chats/links/test_sync_links.py:18-18
Timestamp: 2026-04-02T09:35:03.825Z
Learning: In this repository’s test suite, flake8-aaa/flake8-aaa codes use short two-digit suffixes (e.g., `# noqa: AAA01`), not three-digit variants like `AAA001`. If you see `# noqa: AAA01` in a test file (e.g., when the Act step is performed via a pytest fixture rather than inline code), treat it as valid and intentional—do not flag it as an invalid/no-longer-needed noqa, and do not suggest removing it even if Ruff reports `RUF102`, since `AAA` is configured under `tool.ruff.lint.external` and these noqa directives are expected to be preserved.
Applied to files:
tests/unit/auth/test_extension_framework.pytests/e2e/conftest.pytests/unit/http/test_jwt.pytests/unit/resources/integration/test_installations_token.py
📚 Learning: 2026-02-02T13:05:41.144Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 201
File: tests/unit/resources/accounts/mixins/test_activatable_mixin.py:132-139
Timestamp: 2026-02-02T13:05:41.144Z
Learning: In the mpt-api-python-client repository, tests are configured to use pytest asyncio mode auto, which auto-detects async test functions and runs them without requiring pytest.mark.asyncio. Reviewers should rely on this behavior for all Python test files under tests/, and avoid adding unnecessary asyncio markers in async tests. Ensure test files in tests/ adhere to this convention unless a specific test requires an explicit marker.
Applied to files:
tests/unit/auth/test_extension_framework.pytests/e2e/conftest.pytests/unit/http/test_jwt.pytests/unit/resources/integration/test_installations_token.py
📚 Learning: 2026-01-08T08:34:05.465Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 183
File: tests/e2e/catalog/price_lists/conftest.py:29-30
Timestamp: 2026-01-08T08:34:05.465Z
Learning: In end-to-end tests (e.g., tests/e2e/...), reuse existing API resources for read-only operations and safe mutations to speed up test execution. Reserve isolated fixtures (e.g., created_price_list) for destructive tests that require per-test creation and cleanup. Ensure tests document when a fixture creates/destroys data and clearly indicate which operations are destructive, so tests stay fast and properly isolated.
Applied to files:
tests/e2e/conftest.py
📚 Learning: 2026-01-08T23:38:19.565Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 186
File: tests/e2e/billing/ledger/charge/test_sync_ledger_charge.py:33-39
Timestamp: 2026-01-08T23:38:19.565Z
Learning: In Python e2e tests under tests/e2e, hardcoded external IDs (e.g., INV12345) are intentional because they come from seeded test data. Ensure the test data seeds consistently include these IDs; if seeds change, update tests accordingly. Prefer using a named constant for such IDs (e.g., INV_EXTERNAL_ID) and document the dependency on seed data to avoid brittle tests.
Applied to files:
tests/e2e/conftest.py
📚 Learning: 2026-04-02T16:26:46.138Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 275
File: tests/e2e/exchange/currencies/conftest.py:43-43
Timestamp: 2026-04-02T16:26:46.138Z
Learning: In this repo’s Python E2E conftest files, line-specific flake8/wemake suppressions like `# noqa: WPS421` placed inline on individual statements (e.g., on specific `print()` calls in teardown blocks) are intentional and acceptable. When reviewing, avoid suggesting converting these to a file-wide ignore (e.g., adding per-file ignores under `[tool.flake8]` in `pyproject.toml`), since that would over-suppress WPS421 for the entire file. Also treat Ruff `RUF102` about an unknown rule code as expected here when suppressing `WPS421` from wemake-python-styleguide.
Applied to files:
tests/e2e/conftest.py
📚 Learning: 2026-04-06T09:03:34.466Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 277
File: tests/e2e/extensibility/extensions/conftest.py:37-40
Timestamp: 2026-04-06T09:03:34.466Z
Learning: In this repo’s E2E test conftest teardown fixtures, it is intentional to catch `MPTAPIError` broadly (not just 404) and to print a diagnostic message instead of re-raising. This avoids failing the test suite during teardown when a resource cannot be deleted in some scenarios. Do not recommend narrowing the `except` clause in these teardown blocks (e.g., checking `error.status == 404`), unless the fixture is clearly not intended as an E2E teardown diagnostic.
Applied to files:
tests/e2e/conftest.py
🪛 ast-grep (0.43.0)
tests/unit/auth/test_extension_framework.py
[info] 20-20: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: Security best practice.
(use-jsonify)
tests/unit/http/test_jwt.py
[info] 13-13: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: Security best practice.
(use-jsonify)
🪛 Ruff (0.15.17)
tests/unit/auth/test_extension_framework.py
[error] 14-14: Possible hardcoded password assigned to: "SECRET"
(S105)
mpt_api_client/__init__.py
[warning] 9-9: Invalid rule code in # noqa: WPS410
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
mpt_api_client/auth/__init__.py
[warning] 4-4: Invalid rule code in # noqa: WPS410
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
mpt_api_client/http/__init__.py
[warning] 11-11: Invalid rule code in # noqa: WPS410
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
tests/e2e/conftest.py
[warning] 72-72: Invalid rule code in # noqa: WPS421
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
tests/unit/http/test_jwt.py
[error] 36-36: Possible hardcoded password assigned to: "token"
(S105)
tests/unit/resources/integration/test_installations_token.py
[error] 41-41: Possible hardcoded password assigned to: "token"
(S105)
[error] 55-55: Possible hardcoded password assigned to: "token"
(S105)
[error] 68-68: Possible hardcoded password assigned to: "token"
(S105)
🔇 Additional comments (19)
mpt_api_client/auth/extension_framework.py (7)
43-65: LGTM!
67-72: LGTM!
74-100: LGTM!
102-134: LGTM!
136-150: LGTM!
152-168: LGTM!
1-11: Not a concern:typing.overrideis appropriate for this project's Python 3.12+ requirement.This project declares
requires-python = ">=3.12,<4"inpyproject.toml, makingtyping.overridedirectly available without fallback imports. The code is correct as written.> Likely an incorrect or invalid review comment.tests/unit/auth/test_extension_framework.py (5)
1-26: LGTM!
28-57: LGTM!
59-121: LGTM!
123-225: LGTM!
227-268: LGTM!mpt_api_client/__init__.py (2)
1-5: LGTM!Also applies to: 10-16
9-9: Ruff external-rule configuration for WPS is already correctly configured.The
# noqa: WPS410directive on line 9 is preserved, and Ruff is already configured withexternal = ["AAA", "WPS"]intool.ruff.lint, which properly treats these external flake8 families as external. No RUF102 violations are present in the codebase.mpt_api_client/auth/__init__.py (1)
1-8: LGTM!mpt_api_client/http/__init__.py (1)
4-8: LGTM!Also applies to: 15-18
tests/e2e/conftest.py (1)
19-49: LGTM!Also applies to: 51-132
tests/unit/http/test_jwt.py (1)
13-49: LGTM!tests/unit/resources/integration/test_installations_token.py (1)
15-70: LGTM!
Introduce an Authentication provider abstraction on top of httpx.Auth and rework the HTTP clients to use it, so authentication is no longer hardcoded to a single long-lived bearer token. Providers live in a dedicated mpt_api_client.auth package. Two providers are included: - BearerTokenAuthentication (auth.base): single long-lived bearer token, equivalent to the previous default behavior. - ExtensionFrameworkAuthentication (auth.extension_framework): short-lived installation or account-scoped token. It delegates the token call to the new InstallationsTokenService, refreshes proactively before the JWT exp (default 60s leeway) and reactively on 401. Pass account_id for an account-scoped token; use one provider instance per scope. Because providers subclass httpx.Auth, a single implementation works for both HTTPClient and AsyncHTTPClient. The clients hand their resolved configuration to the provider through Authentication.configure(); ExtensionFrameworkAuthentication uses it to build its dedicated token client instead of deriving it from the in-flight request. The http layer references Authentication only under TYPE_CHECKING, so it keeps no runtime dependency on a concrete auth module. The POST /public/v1/integration/installations/-/token call lives solely in InstallationsTokenService (client.integration.installations_token()), and http.jwt decodes the unverified exp claim to drive proactive refresh. Add unit coverage (tests/unit/auth, http/test_jwt, integration token service) and e2e coverage for the installation token and extension-framework auth flow (skipped unless MPT_API_TOKEN_EXTENSION is set). Breaking change: HTTPClient, AsyncHTTPClient, MPTClient.from_config and AsyncMPTClient.from_config now require an authentication provider instead of the previous api_token argument. The MPT_API_TOKEN environment variable is no longer read. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
albertsola
force-pushed
the
feature/MPT-21532-authentication-providers
branch
from
b9b9e3c to
5280c10
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/auth/test_extension_framework.py`:
- Around line 84-106: The test
`test_extension_framework_retries_non_idempotent_request_on_unauthorized` and
the similar test(s) at lines 184-208 are verifying that POST requests are
automatically retried after receiving a 401 response. This is problematic
because automatic retries of non-idempotent methods like POST can cause
duplicate writes if the first request reached business logic before being
rejected. Remove or modify these tests to verify that non-idempotent requests
(POST, PUT, PATCH, DELETE) are NOT automatically retried on 401 errors. Only
idempotent methods (GET, HEAD, OPTIONS) should be retried, or non-idempotent
methods should require an explicit idempotency key before attempting retry
logic. Update all affected test cases at the mentioned locations to align with
this safer behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: fd68f21e-3f1b-4b9f-aaa9-7a9b1a696f97
📒 Files selected for processing (37)
.env.sampleREADME.mddocs/architecture.mddocs/e2e_tests.mddocs/rql.mddocs/usage.mdmpt_api_client/__init__.pympt_api_client/auth/__init__.pympt_api_client/auth/base.pympt_api_client/auth/extension_framework.pympt_api_client/auth/jwt.pympt_api_client/http/async_client.pympt_api_client/http/client.pympt_api_client/mpt_client.pympt_api_client/resources/integration/installations_token.pympt_api_client/resources/integration/integration.pypyproject.tomltests/e2e/auth/__init__.pytests/e2e/auth/conftest.pytests/e2e/auth/test_async_extension_framework.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/conftest.pytests/e2e/integration/installations_token/__init__.pytests/e2e/integration/installations_token/conftest.pytests/e2e/integration/installations_token/test_async_installations_token.pytests/e2e/integration/installations_token/test_sync_installations_token.pytests/e2e/test_access.pytests/unit/auth/__init__.pytests/unit/auth/test_base.pytests/unit/auth/test_extension_framework.pytests/unit/auth/test_jwt.pytests/unit/conftest.pytests/unit/http/test_async_client.pytests/unit/http/test_client.pytests/unit/resources/integration/test_installations_token.pytests/unit/resources/integration/test_integration.pytests/unit/test_mpt_client.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
softwareone-platform/mpt-extension-skills(manual)
✅ Files skipped from review due to trivial changes (8)
- tests/unit/conftest.py
- docs/e2e_tests.md
- .env.sample
- docs/rql.md
- README.md
- docs/architecture.md
- pyproject.toml
- docs/usage.md
🚧 Files skipped from review as they are similar to previous changes (18)
- tests/e2e/integration/installations_token/test_async_installations_token.py
- tests/e2e/integration/installations_token/conftest.py
- tests/unit/auth/test_base.py
- tests/unit/resources/integration/test_integration.py
- tests/unit/http/test_client.py
- tests/e2e/auth/conftest.py
- mpt_api_client/resources/integration/integration.py
- tests/e2e/auth/test_async_extension_framework.py
- tests/unit/http/test_async_client.py
- mpt_api_client/http/client.py
- tests/e2e/test_access.py
- tests/e2e/integration/installations_token/test_sync_installations_token.py
- mpt_api_client/resources/integration/installations_token.py
- mpt_api_client/auth/base.py
- mpt_api_client/http/async_client.py
- tests/unit/test_mpt_client.py
- mpt_api_client/mpt_client.py
- mpt_api_client/auth/extension_framework.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved
Files:
tests/unit/auth/test_jwt.pympt_api_client/auth/jwt.pytests/e2e/auth/test_sync_extension_framework.pympt_api_client/__init__.pympt_api_client/auth/__init__.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/resources/integration/test_installations_token.py
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdWorking protocol for any task in this repository:
- Identify the task type and select only the local repository files that are relevant to that task.
- Read only those relevant local files before making changes.
- If any selected local file references shared standards or shared operational guidance that are relevant to the same task, read those shared documents too before proceeding.
- Treat repository-local documents as repository-specific additions, restrictions, or overrides to shared guidance.
- If a repository-local rule conflicts with a shared rule, the local repository rule takes precedence.
Python API client for the SoftwareONE Marketplace Platform (MPT) API. Provides synchronous
(MPTClient) and asynchronous (AsyncMPTClient) clients built on httpx, with typed
resource services, mixin-based HTTP operations, and an RQL query builder.Documentation Reading Order
When applicable, read the repository documentation in this order:
README.md— repository overview, quick start, and documentation mapdocs/usage.md— installation, configuration, Python usage examples, and supported Docker-based commandsdocs/architecture.md— layered architecture, directory structure, and key abstractionsdocs/local-development.md— Docker-only setup and execution modeldocs/testing.md— repository-specific testing strategy and command mappingdocs/contributing.md— repository-specific workflow and links to shared standardsdocs/documentation.md— repository-specific documentation rulesdocs/unit_tests.md— unit test structure and guidancedocs/e2e_tests.md— end-to-end test setup and executionThen inspect the code paths relevant to the task:
mpt_api_client/mpt_client.py— public sync and async client entry pointsmpt_api_client/http/— HTTP clients, services, query state, and reusable mixinsmpt_api_client/resources/— domain resource groups such as catalog, commerce, billing, and integrati...
Files:
tests/unit/auth/test_jwt.pympt_api_client/auth/jwt.pytests/e2e/auth/test_sync_extension_framework.pympt_api_client/__init__.pympt_api_client/auth/__init__.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/resources/integration/test_installations_token.py
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:10.872Z
Learning: Use a layered architecture with four distinct layers: Client Layer (MPTClient/AsyncMPTClient), Resource Groups, Service Layer with Mixins, and HTTP Transport (HTTPClient/AsyncHTTPClient)
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:10.872Z
Learning: Organize the client codebase into modules by API domain (accounts, audit, billing, catalog, commerce, exchange, helpdesk, integration, notifications, program, spotlight), with each domain exposed as a property on the client
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:19.023Z
Learning: E2E test suites must use `pytest` markers and require live API credentials, so they run outside the default `make test` invocation
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:19.023Z
Learning: E2E tests must be run using `make test args="tests/e2e"` for the full suite or `make test args="tests/e2e/<module>"` for subset testing
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:19.023Z
Learning: Environment variables `MPT_API_BASE_URL`, `MPT_API_TOKEN_VENDOR`, `MPT_API_TOKEN_CLIENT`, `MPT_API_TOKEN_OPERATIONS`, and `MPT_API_TOKEN_EXTENSION` must be configured for E2E tests
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:19.023Z
Learning: Optional ReportPortal integration can be configured using environment variables `RP_API_KEY`, `RP_ENDPOINT`, and `RP_LAUNCH`
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:19.023Z
Learning: E2E test results must be published to ReportPortal at https://report-portal.s1.team/
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:32.315Z
Learning: Install the mpt-api-client package using `pip install mpt-api-client` or `uv add mpt-api-client`
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:32.315Z
Learning: Require Python 3.12+ as a prerequisite for using the mpt-api-client library
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client
Timestamp: 2026-06-16T16:37:32.315Z
Learning: Configure the MPT API client with a base URL from the `MPT_API_BASE_URL` environment variable and an explicit authentication provider
📚 Learning: 2025-12-12T15:02:20.732Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 160
File: tests/e2e/commerce/agreement/attachment/test_async_agreement_attachment.py:55-58
Timestamp: 2025-12-12T15:02:20.732Z
Learning: In pytest with pytest-asyncio, if a test function uses async fixtures but contains no await, declare the test function as def (synchronous) instead of async def. Pytest-asyncio will resolve the async fixtures automatically; this avoids linter complaints about unnecessary async functions. This pattern applies to any test file under the tests/ directory that uses such fixtures.
Applied to files:
tests/unit/auth/test_jwt.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/resources/integration/test_installations_token.py
📚 Learning: 2026-04-02T09:35:03.825Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 269
File: tests/e2e/helpdesk/chats/links/test_sync_links.py:18-18
Timestamp: 2026-04-02T09:35:03.825Z
Learning: In this repository’s test suite, flake8-aaa/flake8-aaa codes use short two-digit suffixes (e.g., `# noqa: AAA01`), not three-digit variants like `AAA001`. If you see `# noqa: AAA01` in a test file (e.g., when the Act step is performed via a pytest fixture rather than inline code), treat it as valid and intentional—do not flag it as an invalid/no-longer-needed noqa, and do not suggest removing it even if Ruff reports `RUF102`, since `AAA` is configured under `tool.ruff.lint.external` and these noqa directives are expected to be preserved.
Applied to files:
tests/unit/auth/test_jwt.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/resources/integration/test_installations_token.py
📚 Learning: 2026-02-02T13:05:41.144Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 201
File: tests/unit/resources/accounts/mixins/test_activatable_mixin.py:132-139
Timestamp: 2026-02-02T13:05:41.144Z
Learning: In the mpt-api-python-client repository, tests are configured to use pytest asyncio mode auto, which auto-detects async test functions and runs them without requiring pytest.mark.asyncio. Reviewers should rely on this behavior for all Python test files under tests/, and avoid adding unnecessary asyncio markers in async tests. Ensure test files in tests/ adhere to this convention unless a specific test requires an explicit marker.
Applied to files:
tests/unit/auth/test_jwt.pytests/e2e/auth/test_sync_extension_framework.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/resources/integration/test_installations_token.py
📚 Learning: 2026-04-16T13:00:41.320Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 303
File: tests/e2e/helpdesk/chats/participants/conftest.py:25-31
Timestamp: 2026-04-16T13:00:41.320Z
Learning: In mpt-api-python-client, do not treat list-wrapped arguments to CreateMixin.create() / AsyncCreateMixin.create() as an error. ResourceData is intentionally typed as Resource | list[Resource] (see mpt_api_client/models/model.py), so create() should accept either a single resource dict or a list of resource dicts (e.g., create([chat_participant_data])) to perform a batch create and return a ModelCollection. Therefore, reviewers should only flag list-wrapped create() arguments when there is evidence they violate the expected API contract beyond this documented batch-create behavior.
Applied to files:
tests/unit/auth/test_jwt.pympt_api_client/auth/jwt.pytests/e2e/auth/test_sync_extension_framework.pympt_api_client/__init__.pympt_api_client/auth/__init__.pytests/e2e/conftest.pytests/unit/auth/test_extension_framework.pytests/unit/resources/integration/test_installations_token.py
📚 Learning: 2026-02-17T10:04:00.873Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 210
File: mpt_api_client/rql/query_builder.py:18-18
Timestamp: 2026-02-17T10:04:00.873Z
Learning: In this repository, Ruff and flake8 with wemake-python-styleguide are used together. Do not remove WPS* noqa directives (e.g., WPS231) even if Ruff flags them as unknown in RUF102. Keep the directives to satisfy flake8 rules; ensure tooling configuration accounts for both linters to avoid false positives.
Applied to files:
mpt_api_client/auth/jwt.pympt_api_client/__init__.pympt_api_client/auth/__init__.py
📚 Learning: 2026-01-08T08:34:05.465Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 183
File: tests/e2e/catalog/price_lists/conftest.py:29-30
Timestamp: 2026-01-08T08:34:05.465Z
Learning: In end-to-end tests (e.g., tests/e2e/...), reuse existing API resources for read-only operations and safe mutations to speed up test execution. Reserve isolated fixtures (e.g., created_price_list) for destructive tests that require per-test creation and cleanup. Ensure tests document when a fixture creates/destroys data and clearly indicate which operations are destructive, so tests stay fast and properly isolated.
Applied to files:
tests/e2e/auth/test_sync_extension_framework.pytests/e2e/conftest.py
📚 Learning: 2026-01-08T23:38:19.565Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 186
File: tests/e2e/billing/ledger/charge/test_sync_ledger_charge.py:33-39
Timestamp: 2026-01-08T23:38:19.565Z
Learning: In Python e2e tests under tests/e2e, hardcoded external IDs (e.g., INV12345) are intentional because they come from seeded test data. Ensure the test data seeds consistently include these IDs; if seeds change, update tests accordingly. Prefer using a named constant for such IDs (e.g., INV_EXTERNAL_ID) and document the dependency on seed data to avoid brittle tests.
Applied to files:
tests/e2e/auth/test_sync_extension_framework.pytests/e2e/conftest.py
📚 Learning: 2026-04-02T16:26:46.138Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 275
File: tests/e2e/exchange/currencies/conftest.py:43-43
Timestamp: 2026-04-02T16:26:46.138Z
Learning: In this repo’s Python E2E conftest files, line-specific flake8/wemake suppressions like `# noqa: WPS421` placed inline on individual statements (e.g., on specific `print()` calls in teardown blocks) are intentional and acceptable. When reviewing, avoid suggesting converting these to a file-wide ignore (e.g., adding per-file ignores under `[tool.flake8]` in `pyproject.toml`), since that would over-suppress WPS421 for the entire file. Also treat Ruff `RUF102` about an unknown rule code as expected here when suppressing `WPS421` from wemake-python-styleguide.
Applied to files:
tests/e2e/conftest.py
📚 Learning: 2026-04-06T09:03:34.466Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 277
File: tests/e2e/extensibility/extensions/conftest.py:37-40
Timestamp: 2026-04-06T09:03:34.466Z
Learning: In this repo’s E2E test conftest teardown fixtures, it is intentional to catch `MPTAPIError` broadly (not just 404) and to print a diagnostic message instead of re-raising. This avoids failing the test suite during teardown when a resource cannot be deleted in some scenarios. Do not recommend narrowing the `except` clause in these teardown blocks (e.g., checking `error.status == 404`), unless the fixture is clearly not intended as an E2E teardown diagnostic.
Applied to files:
tests/e2e/conftest.py
🪛 ast-grep (0.43.0)
tests/unit/auth/test_jwt.py
[info] 13-13: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: Security best practice.
(use-jsonify)
tests/unit/auth/test_extension_framework.py
[info] 20-20: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: Security best practice.
(use-jsonify)
🪛 Ruff (0.15.17)
tests/unit/auth/test_jwt.py
[error] 36-36: Possible hardcoded password assigned to: "token"
(S105)
mpt_api_client/__init__.py
[warning] 9-9: Invalid rule code in # noqa: WPS410
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
mpt_api_client/auth/__init__.py
[warning] 4-4: Invalid rule code in # noqa: WPS410
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
tests/e2e/conftest.py
[warning] 72-72: Invalid rule code in # noqa: WPS421
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
tests/unit/auth/test_extension_framework.py
[error] 14-14: Possible hardcoded password assigned to: "SECRET"
(S105)
tests/unit/resources/integration/test_installations_token.py
[error] 41-41: Possible hardcoded password assigned to: "token"
(S105)
[error] 55-55: Possible hardcoded password assigned to: "token"
(S105)
[error] 68-68: Possible hardcoded password assigned to: "token"
(S105)
🔇 Additional comments (8)
mpt_api_client/auth/jwt.py (1)
1-57: LGTM!tests/unit/auth/test_jwt.py (1)
1-49: LGTM!tests/e2e/auth/test_sync_extension_framework.py (1)
1-10: LGTM!mpt_api_client/__init__.py (1)
1-16: LGTM!mpt_api_client/auth/__init__.py (1)
1-8: LGTM!tests/e2e/conftest.py (1)
10-12: LGTM!Also applies to: 19-132
tests/unit/auth/test_extension_framework.py (1)
1-83: LGTM!Also applies to: 108-183, 210-304
tests/unit/resources/integration/test_installations_token.py (1)
1-70: LGTM!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI-generated PR — Please review carefully.
What
Introduces a generic
Authenticationprovider abstraction on top ofhttpx.Authand reworks the HTTP clients to use it, so authentication is no longer hardcoded to a single long-lived bearer token. Providers live in a dedicatedmpt_api_client.authpackage.Two providers are included:
BearerTokenAuthentication(auth.base) — single long-lived bearer token, equivalent to the previous default behavior.ExtensionFrameworkAuthentication(auth.extension_framework) — short-lived installation or account-scoped token. It delegates the token call to the integrationInstallationsTokenService, refreshes proactively before the JWTexp(default 60s leeway) and reactively on401. Passaccount_idfor an account-scoped token; use one provider instance per scope.Because providers subclass
httpx.Auth, a single implementation works for bothHTTPClientandAsyncHTTPClient.Key design points
authpackage:Authentication/BearerTokenAuthenticationinmpt_api_client/auth/base.py;ExtensionFrameworkAuthenticationinmpt_api_client/auth/extension_framework.py. The low-levelhttpclients type their argument ashttpx.Auth, sohttpno longer depends on a concrete auth module (avoids a layering inversion / import cycle).POST /public/v1/integration/installations/-/tokencall lives inInstallationsTokenService(mpt_api_client/resources/integration/installations_token.py), exposed asclient.integration.installations_token().token(account_id=...).ExtensionFrameworkAuthenticationreuses it instead of building the request itself.mpt_api_client/http/jwt.pydecodes the unverifiedexpclaim to drive proactive refresh.Public API
Breaking change
HTTPClient,AsyncHTTPClient,MPTClient.from_configandAsyncMPTClient.from_confignow require anauthenticationprovider instead of the previousapi_tokenargument. TheMPT_API_TOKENenvironment variable is no longer read.Testing
make check-all(ruff, flake8/wemake, mypy, unit tests) — green; full unit suite passes.tests/unit/auth/(providers),tests/unit/http/test_jwt.py,tests/unit/resources/integration/test_installations_token.py.tests/e2e/auth/(extension-framework auth flow) andtests/e2e/integration/installations_token/(token endpoint). They skip unlessMPT_API_TOKEN_EXTENSIONis set and create an installation to derive a token-mintable account, so they require live credentials to execute.Closes MPT-21532
Authenticationabstraction on top ofhttpx.Authin newmpt_api_client.authpackageBearerTokenAuthenticationfor single long-lived bearer tokens (equivalent to previous default behavior)ExtensionFrameworkAuthenticationfor short-lived installation/account-scoped tokens with proactive refresh (60s default leeway before JWT expiry) and reactive 401-based refreshInstallationsTokenService(sync and async) to centralize token endpoint access atclient.integration.installations_token().token(account_id=...)mpt_api_client/auth/jwt.py) to decode unverified JWT claims for proactive token refresh validationHTTPClientandAsyncHTTPClient) to acceptauthenticationprovider instead ofapi_tokenparameter and callauthentication.configure()with resolved connection detailsMPTClient.from_configandAsyncMPTClient.from_config) to requireauthenticationprovider instead ofapi_tokenMPT_API_TOKEN_EXTENSIONenvironment variable)MPT_API_BASE_URLconfigurationauthenticationprovider is now required forHTTPClient,AsyncHTTPClient,MPTClient.from_config(), andAsyncMPTClient.from_config(). Theapi_tokenparameter andMPT_API_TOKENenvironment variable are no longer supported.